-
Notifications
You must be signed in to change notification settings - Fork 422
CLDR-19192 use a case insensensitive collator for TestAnnotations.TestUniqueness #5276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
CLDR-19192 use a case insensensitive collator for TestAnnotations.TestUniqueness #5276
Conversation
…tUniqueness - use ROOT_SECONDARY collator
macchiati
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have two small suggestions; otherwise looks great.
tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestAnnotations.java
Show resolved
Hide resolved
Should this go in as log known issue, with the actual fixes in CLDR-19189 ? otherwise this can't merge until data fixed |
also fix docs around problems and improve parallel stream
|
@macchiati thanks, but should this be a log known issue until the data is fixed? or leave this PR open awaiting data fix? It can't merge as is, since it fails. |
|
I think it would be good to add as a logged known issue and then we can discuss in the next TC meeting how the TC prefers to resolve this in 49. |
|
OK, great, I will do It and submit this for a review
|
|
Logged known issue is fine for now, but we need to document this as a Known issue on the 48 release page, with pointer to the ticket/PR that fixes it, so that people can cherry pick. |
… secondary/casing changes
|
logknownissue is as follows:
|
tools/cldr-code/src/main/java/org/unicode/cldr/util/CollatorHelper.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| private static Comparator<String> makeNFKC_CF_SECONDARY() { | ||
| private static Comparator<String> makeROOT_INSENSITIVE() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A better name would be CASE_FOLDED (it has nothing to do with root or collation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do that in a later round.
sorry for the noise, trying to move this forward
|
@macchiati nothing fails, so probably we can remove the logknownissue? My mistake, these seem to be known issues still a little unexpeted, i'll need to investigate monday |
CLDR-19192
ALLOW_MANY_COMMITS=true